Theme JSON: remove unused vars in layout class #59938
Conversation
$has_block_gap_support is only used once
| @@ -2729,9 +2729,7 @@ public function get_root_layout_rules( $selector, $block_metadata ) { | |||
| $css .= '.wp-site-blocks > .alignright { float: right; margin-left: 2em; }'; | |||
| $css .= '.wp-site-blocks > .aligncenter { justify-content: center; margin-left: auto; margin-right: auto; }'; | |||
|
|
|||
| $block_gap_value = $this->theme_json['styles']['spacing']['blockGap'] ?? '0.5em'; | |||
There was a problem hiding this comment.
I'm still seeing a fallback in the JS
Should the fallback of '0.5em' be used in the if statement if static::get_property_value returns empty?
There was a problem hiding this comment.
Good question! A couple of considerations:
- Core's
lib/theme.jsonincludes astyles.spacing.blockGapvalue of24px, so in practice, was the0.5emvalue ever being output? I'm curious if the0.5emdefault spacing was ever being displayed visually for anyone 🤔 - I think the
0.5emfallback was originally intended for flex layouts so that on classic themes where there might not be any blockGap value, things like Buttons spacing would still get a visually pleasing default value. In the case of root layout rules, that might not be necessary?
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
There was a problem hiding this comment.
Good catch! ✨
✅ Smoke tested that root layout block spacing rules are still output when settings.spacing.blockGap is either true or false
✅ They are not output when blockGap is set to null
✅ Styles are output correctly, and the previous fallback value was never being used anyway
LGTM, just left a note about whether it's worth adding a one-line comment to flag that blockGap outputs in true and false but not null.
andrewserong
added
the
Global Styles
Remove unused vars: - $block_gap_value is immediately overwritten in the if block - $has_block_gap_support is only used once Co-authored-by: ramonjd <[email protected]> Co-authored-by: andrewserong <[email protected]>
What?
Removing some unused vars in the Theme JSON class.
Noticed while working on #42084
Why?
$has_block_gap_supportis only used once$block_gap_valueis immediately overwritten in the subsequentifblockTesting Instructions
Tests should pass.
On the frontend, there should be no regressions: the block gap value should be correctly output where the ✅ is:
:where(.wp-site-blocks) > * { margin-block-start: ✅ ; margin-block-end: 0; }andbody { --wp--style--block-gap: ✅; }